fix: replace builtin-modules package with node's isBuiltin check #2176
Conversation
| @@ -43,7 +39,6 @@ export const createInstallMissingTypesProvider = () => { | |||
| const missingPackageNames = setSubtract( | |||
| referencedPackageNames, | |||
| new Set(existingPackageNames), | |||
There was a problem hiding this comment.
I wonder, should we give existingPackageNames as a parameter for collectReferencedPackageNames and we could do the check already there, so we didn't need to go through the list again here.
There was a problem hiding this comment.
I did that change in second commit. Instead of using setSubsctract, we never add existingPackageNames to the list in the first place.
It's not exactly what this task / PR was about, so that commit could be removed and I can submit it separately. On the other hand, I think it's pretty simple change.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2176 +/- ##
==========================================
+ Coverage 74.81% 75.85% +1.03%
==========================================
Files 171 170 -1
Lines 7108 7106 -2
Branches 1049 1074 +25
==========================================
+ Hits 5318 5390 +72
+ Misses 1785 1711 -74
Partials 5 5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
PR Checklist
status: accepting prsOverview
There is no need to use external package for this check, when we can use node's own
isBuiltin()check. However, it needed minimum node version to be upgrade to>=18.6.🐙